feat(geometry): Add comprehensive error handling for file loading #1672

abushanisro · 2025-10-23T22:15:59Z

Implements comprehensive error handling for geometry file loading (CHEASE, FBT, EQDSK) as described in issue #1662. Replaces uninformative stack traces with clear, actionable error messages.

Changes Made

New Modules:

geometry_errors.py – Custom exception hierarchy
file_validation.py – File access and format validation
geometry_utils.py – High-level loading functions with error handling

Enhanced Modules:

geometry_loader.py – Integrated validation and error handling
init.py – Exports new exception classes

Key Features
Custom Exception Hierarchy:
GeometryFileError
├── GeometryFileNotFoundError
├── GeometryFileFormatError
├── GeometryFilePermissionError
└── GeometryDataValidationError


**Improved Error Messages:**

Before: `FileNotFoundError: [Errno 2] No such file or directory: 'geometry.chease'`

After:

GeometryFileNotFoundError: Geometry file not found: geometry.chease
Please check:

The file path is correct
The file exists at the specified location
Similar files found: geometry_backup.chease, iter_hybrid.chease
Validation Features:

File existence with similar file suggestions
Permission validation with chmod guidance
Empty file detection
Format-specific data validation

Testing
All 76 geometry tests pass successfully:
bashpytest torax/_src/geometry/tests/ -v

Result: 76 passed, 3 warnings in 3.73s

Two pre-existing test failures in run_simulation_main_test.py are unrelated to this PR (verified on main branch).
Benefits

Clear, actionable error messages
Faster debugging and troubleshooting
Early issue detection
No breaking changes (fully backward compatible)

Checklist

All geometry tests pass (76/76)
No breaking changes to existing API
Code follows project style guidelines
Copyright headers added

Closes #1662

Ready for review. I am happy to address feedback or make any requested revisions.

google-cla · 2025-10-23T22:16:04Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

tamaranorman

This PR seems to move lots of things to 4 spacing not two, can those changes be reverted before us taking a look

tamaranorman · 2025-11-12T17:57:46Z

torax/_src/geometry/circular_geometry.py

This seems to just be formatting changes can they be reverted so that any key changes are highlighted

@tamaranorman Thank you for this feedback!

tamaranorman · 2025-11-12T17:58:10Z

torax/_src/geometry/geometry.py

Similar to this

Nush395 · 2025-12-01T16:04:39Z

hi @abushanisro, are you still interesting in working on this PR? Some of the geometry modules have moved around so it might need a refresh as well as addressing Tamara's points if so.

abushanisro · 2025-12-01T20:56:00Z

Will work on it,let me fix the spacing and get back have some doubts on the modules

jcitrin

The geometry files have undergone some renaming and moving. Please sync to head before triggering re-review.

General comment: many changes are purely formatting changes that are not necessary and mask the meaningful changes in the PR, making review very difficult. Please limit changes to the meaningful ones that actually implement the new features.

jcitrin · 2025-12-07T00:29:06Z

torax/_src/geometry/file_validation.py

+        if similar_files:
+            error_msg += "\n\nSimilar files found in directory:\n  - " + "\n  - ".join(
+                similar_files
+            )


This will cause a lot of spam. Can remove the "similar_files" subfeature

jcitrin · 2025-12-07T00:37:14Z

torax/_src/geometry/geometry_utils.py

+    file_validation.validate_geometry_data(data, file_type, "geometry_data")
+
+
+def get_geometry_info(file_path: Union[str, Path]) -> dict[str, Any]:


Where is this used anywhere?

This function was part of my initial implementation but isnt actually
used anywhere. I've removed the entire function and the associated Any type import.
The core error handling functionality is provided by validate_geometry_data() and the
loader functions (load_chease_file(), load_fbt_file(), load_eqdsk_file())

jcitrin · 2025-12-07T00:40:55Z

torax/_src/geometry/geometry_utils.py

+    # Determine likely file type from extension
+    extension = path.suffix.lower()
+    likely_type = "unknown"
+    if extension in [".chease", ".txt"]:
+        likely_type = "chease"
+    elif extension in [".fbt", ".dat", ".mat"]:
+        likely_type = "fbt"
+    elif extension in [".eqdsk", ".geqdsk"]:
+        likely_type = "eqdsk"


This isn't useful since e.g. in practice any of them could be .txt or .data, and thus this classification could be misleading. Recommend simply removing the "likely_type" subfeature

Removed the likely_type logic and the entire get_geometry_info() function

Implements comprehensive error handling infrastructure for geometry file loading (CHEASE, FBT, EQDSK) as described in issue google-deepmind#1662. New modules: - geometry_errors.py: Custom exception hierarchy for geometry file errors - file_validation.py: File access and format validation utilities - geometry_utils.py: High-level loading functions with error handling - __init__.py: Export exception classes for external use Key features: - Custom exception hierarchy (GeometryFileError and subclasses) - File existence validation with helpful error messages - Permission validation - Empty file detection - Format-specific data validation for CHEASE, FBT, and EQDSK files Benefits: - Clear, actionable error messages instead of generic stack traces - Faster debugging and troubleshooting - Early issue detection - Fully backward compatible (no breaking changes) Addresses reviewer feedback: - Removed 'similar_files' suggestion feature (avoided log spam) - Removed unused get_geometry_info() function - Removed misleading 'likely_type' classification logic Closes google-deepmind#1662

abushanisro · 2025-12-16T11:42:30Z

@jcitrin Thank you for the detailed feedback! I've completely reworked the PR to
address all your concerns

jcitrin

In addition to these comments, please sync to head. The failing tests should then pass

jcitrin · 2026-01-02T12:13:19Z

torax/_src/geometry/__init__.py


 """This package contains functionality related to geometry."""
+
+# Export custom exception classes for external use


Putting these classes into the init here does not truly expose them as an external API since geometry is still under _src. For external API they would need to go to torax init

However, I don't think they need to be external API. I would remove this change to geometry init, and just import them directly from geometry_errors in all the places where they are used

jcitrin · 2026-01-02T12:17:28Z

torax/_src/geometry/file_validation.py

+    # Validate file type
+    if validate_format:
+        expected_extensions = {
+            "chease": [".chease", ".txt", ".mat2cols"],
+            "fbt": [".fbt", ".dat", ".mat"],
+            "eqdsk": [".eqdsk", ".geqdsk"],
+        }
+
+        if file_type.lower() in expected_extensions:
+            valid_ext = path.suffix.lower(
+            ) in expected_extensions[file_type.lower()]
+            if not valid_ext:
+                logger.warning(
+                    "File extension '%s' unusual for %s files. Expected: %s",
+                    path.suffix,
+                    file_type.upper(),
+                    expected_extensions[file_type.lower()],
+                )


This is still similar to the "likely_type" feature that was removed. Just remove all of this, it's not needed since it can cause warning spam on valid files.

jcitrin · 2026-01-02T12:32:28Z

torax/_src/geometry/file_validation.py

+logger = logging.getLogger(__name__)
+
+
+def validate_file_access(


I don't see this function being used anywhere. I recommend moving the function to _src/geometry/geometry_loader, making it a private function _validate_file_access, and then calling it from within load_geo_data

jcitrin · 2026-01-02T12:35:45Z

torax/_src/geometry/file_validation.py

+    return path
+
+
+def validate_geometry_data(data: dict, file_type: str, file_path: str) -> None:


can remove the entire function validate_geometry_data and as a first pass, just call the validators from within chease.py, eqdsk.py, fbt.py after load_geo_data

jcitrin · 2026-01-02T12:36:00Z

torax/_src/geometry/file_validation.py

+        raise geometry_errors.GeometryDataValidationError(error_msg)
+
+
+def _validate_chease_data(data: dict, issues: List[str]) -> None:


remove from here and put into chease.py. It can then be called directly after geometry_loader.load_geo_data is used

jcitrin · 2026-01-02T12:36:50Z

torax/_src/geometry/file_validation.py

+            )
+
+
+def _validate_fbt_data(data: dict, issues: List[str]) -> None:


no need for this. We already have a _validate_fbt_data inside fbt.py

jcitrin · 2026-01-02T12:37:59Z

torax/_src/geometry/file_validation.py

+            )
+
+
+def _validate_eqdsk_data(data: dict, issues: List[str]) -> None:


remove from here and move to eqdsk.py. It can be called just before _validate_eqdsk_cocos11 which is called after load_geo_data

jcitrin · 2026-01-02T12:40:48Z

torax/_src/geometry/file_validation.py

see rest of comments here. In the end I don't think you will need this new file. All functions can be moved to existing modules

jcitrin · 2026-01-02T12:41:38Z

torax/_src/geometry/geometry_utils.py

Can remove all of this. All new functionality will be within geometry_loader.py, fbt.py, chease.py, eqdsk.py

abushanisro force-pushed the feat/geometry-error-handling branch from dd16ea5 to ba35356 Compare October 23, 2025 22:25

tamaranorman reviewed Nov 12, 2025

View reviewed changes

jcitrin reviewed Dec 7, 2025

View reviewed changes

abushanisro force-pushed the feat/geometry-error-handling branch from ba35356 to 0ff104f Compare December 16, 2025 11:39

jcitrin reviewed Jan 2, 2026

View reviewed changes

jcitrin mentioned this pull request Jan 2, 2026

Improve geometry file loading error handling #1753

Closed

abushanisro closed this Jan 5, 2026

		file_validation.validate_geometry_data(data, file_type, "geometry_data")


		def get_geometry_info(file_path: Union[str, Path]) -> dict[str, Any]:


		"""This package contains functionality related to geometry."""

		# Export custom exception classes for external use

		logger = logging.getLogger(__name__)


		def validate_file_access(

		return path


		def validate_geometry_data(data: dict, file_type: str, file_path: str) -> None:

		raise geometry_errors.GeometryDataValidationError(error_msg)


		def _validate_chease_data(data: dict, issues: List[str]) -> None:

		)


		def _validate_fbt_data(data: dict, issues: List[str]) -> None:

		)


		def _validate_eqdsk_data(data: dict, issues: List[str]) -> None:

feat(geometry): Add comprehensive error handling for file loading #1672

feat(geometry): Add comprehensive error handling for file loading #1672

Uh oh!

Conversation

abushanisro commented Oct 23, 2025

Result: 76 passed, 3 warnings in 3.73s

Uh oh!

google-cla bot commented Oct 23, 2025

Uh oh!

tamaranorman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nush395 commented Dec 1, 2025

Uh oh!

abushanisro commented Dec 1, 2025

Uh oh!

jcitrin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abushanisro commented Dec 16, 2025

Uh oh!

jcitrin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants